Refactor default constants: replace strings with enums, and relocate into relevant classes #92
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AndrewSazonov
added
the
[scope] maintenance
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
AndrewSazonov
force-pushed
the
30-refactor-default-constants
branch
from
381ac16 to
fac8ed2
Compare
AndrewSazonov
force-pushed
the
30-refactor-default-constants
branch
from
fac8ed2 to
7801bf8
Compare
rozyczko
reviewed
Sep 5, 2025
| from .minimizers.minimizer_base import FitResults | ||
|
|
||
| if TYPE_CHECKING: | ||
| from .minimizers.minimizer_base import FitResults |
There was a problem hiding this comment.
I made this change because Ruff showed a TC001 warning (“Move application imports used only for typing into a TYPE_CHECKING block”) after I turned on the type-checking rules 🙃
rozyczko
reviewed
Sep 5, 2025
| (ScatteringTypeEnum.TOTAL, BeamModeEnum.CONSTANT_WAVELENGTH): ['r (Å)', 'G(r) (Å)'], | ||
| (ScatteringTypeEnum.TOTAL, BeamModeEnum.TIME_OF_FLIGHT): ['r (Å)', 'G(r) (Å)'], | ||
| } | ||
|
|
rozyczko
reviewed
Sep 5, 2025
| 'I', # Import sorting issues (e.g., unsorted imports) | ||
| 'S', # Security-related issues (e.g., use of insecure functions or libraries) | ||
| 'W', # General PEP 8 warnings (e.g., lines too long, trailing whitespace) | ||
| 'TCH', # Type checking issues (e.g., incompatible types, missing type annotations) |
There was a problem hiding this comment.
We can do static type checking code analysis with ruff? Nice! Probably worth extending to other projects.
There was a problem hiding this comment.
Yeah, Ruff has some basic type-checking support - it already helped catch one or two issues, and that's better than nothing. But we'll still need a more advanced solution, like typeguard or similar.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #30